Skip to content

Webkitdebugger for new node debugger protocol#1483

Merged
paulvanbrenk merged 9 commits intomicrosoft:masterfrom
paulvanbrenk:webkitdebugger15.1
Mar 1, 2017
Merged

Webkitdebugger for new node debugger protocol#1483
paulvanbrenk merged 9 commits intomicrosoft:masterfrom
paulvanbrenk:webkitdebugger15.1

Conversation

@paulvanbrenk
Copy link
Copy Markdown
Contributor

Node will stop supporting the old debugger protocol.

This PR adds uses the webkit debugger, which already exists in VS, which supports the new V8 debugger protocol, which is expected to be the only supported in Node V8.

Note: this needs localization so the new option work in other languages than 'en', and needs some bugfixes in the WebKit debugger.

Copy link
Copy Markdown
Member

@billti billti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple comments. Have you tested this built and applied to VS 2015 Update 3?

internal HierarchyNode FindNodeByFullPath(string name) {
Site.GetUIThread().MustBeCalledFromUIThread();

if (name.StartsWith("mdha:", StringComparison.OrdinalIgnoreCase)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's special about this prefix? Can you add a comment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found a better solution.

_attached = true;
}

public NodeDebugger(string exe, string script, string dir, string env, string interpreterOptions, NodeDebugOptions debugOptions, ushort? debuggerPort = null, bool createNodeWindow = true)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is nearly 200 chars wide now. Please leave it wrapped as it originally was so it is readable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

_webkitDebugger.Checked = page.UseWebKitDebugger;

#if !DEV15
_webkitDebugger.Enabled = false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does this option appear at all on < Dev15? Or does it appear and is disabled?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it appears but is disabled. I can hide it, but then there is a weird gap between the normal options and the survey options.

<data name="_surveyNewsCheckCombo.Items" xml:space="preserve">
<value>Never</value>
<data name="_webkitDebugger.Text" xml:space="preserve">
<value>Use the new NodeJs debugger protocol (for NodeJs v7.5+)</value>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually works from Node.js v6.8.0 onwards on Windows.

return IntPtr.Zero;
}

var guiSize = Marshal.SizeOf(typeof(Guid));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"guidSize"?

@paulvanbrenk paulvanbrenk merged commit 9d24174 into microsoft:master Mar 1, 2017
@paulvanbrenk paulvanbrenk deleted the webkitdebugger15.1 branch April 22, 2017 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants